Preserve cask choice zeroes#22401
Conversation
- Keep `attributeSetting: 0` in internal API cask artifacts because installer choices need zero as a meaningful value. - Allow `Utils.deep_compact_blank` callers to opt out of dropping zeroes while preserving existing default behaviour.
There was a problem hiding this comment.
Pull request overview
This PR fixes internal API cask artifact serialization so that installer choice settings with attributeSetting: 0 are preserved (since 0 is a meaningful value for macOS installer choices), by making Utils.deep_compact_blank optionally retain numeric zeroes.
Changes:
- Add a
compact_zero:keyword option toUtils.deep_compact_blank(defaulting to current behavior of compacting zeroes). - Preserve zero values when compacting/serializing
raw_artifactsinHomebrew::API::CaskStruct#serialize. - Add a regression test ensuring
attributeSetting: 0survives serialization in cask artifacts.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
Library/Homebrew/utils.rb |
Adds compact_zero: option to allow callers to preserve numeric zeroes while deep-compacting. |
Library/Homebrew/api/cask_struct.rb |
Ensures raw_artifacts serialization retains 0 values (notably attributeSetting: 0) while still compacting blanks elsewhere. |
Library/Homebrew/test/api/cask_struct_spec.rb |
Adds coverage to prevent regression for zero-valued installer choice settings in serialized artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
@MikeMcQuaid I wonder if not stripping out 0 values everywhere would be an okay choice. I know it's used for things like bottle rebuild and revision, but adding "revision": 0 to a whole bunch of formulae might not increase the compressed size very much... might be worth a try
|
@Rylan12 Yeh, if we encounter more bugs that's an option but would rather still omit for now. |
Fixes #22400
attributeSetting: 0in internal API cask artifacts because installer choices need zero as a meaningful value.Utils.deep_compact_blankcallers to opt out of dropping zeroes while preserving existing default behaviour.brew lgtm(style, typechecking and tests) with your changes locally?